Skip to content

Stop the validator rejecting pipelines that run correctly (#104) - #448

Merged
jeremymanning merged 1 commit into
mainfrom
fix/validator-false-positives
Aug 2, 2026
Merged

Stop the validator rejecting pipelines that run correctly (#104)#448
jeremymanning merged 1 commit into
mainfrom
fix/validator-false-positives

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Phase 1 of the catalogue repair: the validator, not the examples.

orchestrator validate rejected 108 of 117 examples. Only 23 actually fail
to compile when run. The gate was measuring the validator's strictness rather
than whether a pipeline works.

The decisive case — one pipeline, three ways to name a parameter, all three
rendering identically at run time:

form validate run rendered
{{ topic }} rc=0 rc=0 abc
{{ parameters.topic }} rc=2 rc=0 abc
{{ inputs.topic }} rc=2 rc=0 abc

Four causes

Three environments, three filter sets. Validator 56, compiler 61, runtime
70 — each built its own. {{ title | slugify }} was "Unknown filter" in a
pipeline that renders it fine, and fixing only the validator moved the failure
to truncate_words in the compiler. All three now come from
create_pipeline_environment, sourced from the runtime's registry.

parameters. / inputs. unrecognised. The data-flow validator read
parameters as a task id and reported an undefined task by that name.

A declared parameter without a default was reported undefined. It is
declared; only its value is deferred to -i name=value. A pipeline was told
Undefined variable: 'output_path' about the name it had just declared.

A hardcoded list of twelve output field names. Anything else was "does not
produce output" — asserting knowledge the validator does not have about a model
or <AUTO> step. Now an error only when the task declared its outputs, a
warning otherwise, and the warning still records the data-flow edge.

Results

metric before after
examples validating 9 / 117 18 / 117
raw validation errors 1654 836
distinct errors 335 198
blocking suite 562 573 passed, 0 failed

My prediction was wrong, by a lot

I estimated Phase 1 would unblock ~85 files. It unblocked 9.

The inference was bad: "85 examples compile at run time" does not mean
validate has only one objection to each. Most files carry several independent
error classes, so removing one class halves the error volume without moving
the file count. I conflated two different measurements.

Mutation testing

Each fix reverted against the new tests:

mutation result
envs build their own filter sets again 6 failed
drop parameters/inputs namespaces 2 failed
require a default to count as declared 1 failed
output allowlist back to hard error 1 failed

The last one survived at first. I had changed the severity and the
message, and my test asserted the old message string was absent — which the
reverted path no longer emitted either. It tested wording, not behaviour. The
test now asserts the pipeline compiles.

What remains is real, and I checked rather than assumed

  • output_path (92 occurrences, 16 files) is genuinely undeclared. Those
    pipelines never declare it and the runtime fails identically:
    'output_path' is undefined. The validator is correct.
  • execution (97) — the runtime does not populate it either, so
    data_flow_validator.py:485 accepting it is the bug: too permissive. Fixing
    that makes more things fail, which is why it is deliberately not in this PR.
  • Loop variables (item, loop, row, col_data) unclassified.
  • 17 files missing tools, 13 schema, 6 YAML syntax, 6 model, 4 file-inclusion.

Nothing under examples/ is modified or deleted. Full notes in
notes/example-catalog-diagnosis.md.

`orchestrator validate` rejected 108 of 117 examples. Only 23 of them actually
fail to compile when run. The gate was measuring the validator's strictness
rather than whether a pipeline works, and every false rejection told an author
their working pipeline was broken.

The decisive case -- one pipeline, three ways to name a parameter, all three
rendering identically at run time:

    {{ topic }}              validate rc=0   run rc=0   "abc"
    {{ parameters.topic }}   validate rc=2   run rc=0   "abc"
    {{ inputs.topic }}       validate rc=2   run rc=0   "abc"

Four causes, each mutation-tested:

**Three environments, three filter sets.** The template validator knew 56
filters, the YAML compiler 61, the runtime 70. Each built its own, so
`{{ title | slugify }}` was "Unknown filter" -- in a pipeline that renders it
fine. Fixing only the validator moved the failure to `truncate_words` in the
compiler. They are all built by `create_pipeline_environment` now, from the
runtime's registry, so they cannot drift apart again.

**`parameters.` / `inputs.` unrecognised.** The data-flow validator went
further and read `parameters` as a *task id*, reporting an undefined task by
that name.

**A declared parameter without a default was reported undefined.** It is
declared; only its value is deferred to `-i name=value`. The validator
registered only parameters carrying a default, so a pipeline was told
`Undefined variable: 'output_path'` about the name it had just declared.

**A hardcoded list of twelve output field names.** Any other field was "does
not produce output". A model or `<AUTO>` step's output shape is not statically
knowable, so this asserted knowledge the validator did not have. It is now an
error only when the task declared its outputs, and a warning otherwise -- the
warning still records the data-flow edge.

Examples validating: 9/117 -> 18/117. Raw validation errors: 1654 -> 836.
Blocking suite: 562 -> 573 passed, 0 failed.

I predicted this would unblock ~85 files and it unblocked 9. "Compiles at run
time" does not mean `validate` has only one objection; most files carry several
independent error classes, so removing one halves the error volume without
moving the file count. What remains was spot-checked and is real: `output_path`
is genuinely undeclared in the files that reference it, and the runtime fails
on it identically. Notes in notes/example-catalog-diagnosis.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremymanning

Copy link
Copy Markdown
Member Author

CI verification

9/9 green.

Legacy tally against the current post-merge main run (30767725710, which
already includes #447):

main #448 delta
failed 473 473 0
passed 1847 1847 0
errors 189 189 0
skipped 200 200 0
warnings 11 11 0
deselected 661 672 +11

Identical on every axis, with the +11 deselected exactly the 11 tests this PR
adds to the blocking layer (562 -> 573), which the legacy job runs the
complement of.

That is worth a sentence rather than a shrug. This PR changes five production
files, including the environment that every template in the system is rendered
through and the severity of a whole class of data-flow error. Two and a half
thousand legacy tests did not move by one, and the blocking layer went from 562
to 573 passed with nothing failing.

Two readings of that, and I do not think the evidence separates them:

  1. The change really is confined to rejecting less, so nothing that passed
    before had its behaviour altered.
  2. The legacy layer has so little coverage of the validation path that it would
    not have noticed either way.

The blocking suite plus the four mutation tests are what I am actually relying
on here. The legacy zero is consistent with the change being safe; it is not by
itself evidence of it.

@jeremymanning
jeremymanning merged commit 61ee259 into main Aug 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant